-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Add test for custom attribute with generic enum array argument #123445
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
this is meant to land after #123439 as a test for the mono crash. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds both a Mono runtime fix and a regression test for a crash that occurred when loading custom attributes with generic enum constructor arguments involving function pointer arrays.
Changes:
- Fixed Mono runtime crash in custom attribute decoding for generic enum types with function pointer arrays
- Added regression test to prevent future occurrences of this issue
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/mono/mono/metadata/custom-attrs.c | Defers tklass initialization for MONO_TYPE_GENERICINST types and updates assertion to handle generic instance cases, preventing crash during custom attribute value loading |
| src/libraries/System.Runtime/tests/System.Reflection.Tests/CustomAttributeTests.cs | Adds CustomAttributeCtor_WithGenericEnumArgument_DecodesCorrectly test to verify custom attributes with generic enum arguments involving function pointer arrays decode without crashing |
| var arg = attr.ConstructorArguments.Single(); | ||
| Assert.Equal(typeof(CustomAttributeTypedArgument), arg.GetType()); |
Copilot
AI
Jan 31, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding more comprehensive assertions to validate the actual argument value and type, similar to the BoxedEnumAttributes() test pattern on lines 259-277. For example, you could assert that arg.ArgumentType equals the expected generic enum type typeof(GenericClassForEnum<delegate*<void>[]>.E) and verify the value is the default enum value (0). This would provide stronger regression coverage beyond just ensuring the code doesn't crash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot apply this feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhanced the test assertions in commit c7189a0. The test now validates:
- ArgumentType matches the expected generic enum type
GenericClassForEnum<delegate*<void>[]>.E - Value is the default enum value (0)
This provides stronger regression coverage beyond just ensuring the code doesn't crash.
src/libraries/System.Runtime/tests/System.Reflection.Tests/CustomAttributeTests.cs
Outdated
Show resolved
Hide resolved
|
@copilot this doesn't build because of |
Fixed in commit d05fc87. Marked the test method as |
|
@copilot test failure: |
|
I'm guessing the mono failures are #90308 now |
Do we need a function pointer array for this regression test? Wouldn't we hit the problem with something like public unsafe GenericEnumAttributeWithFunctionPointer(GenericClassForEnum<int[]>.E e) { }too? It would be enough to hit the System.Reflection.Metadata bug. Function pointers might hit more bonus bugs too, but I'd cross that bridge when we get there. |
538dd50 to
52f4a3f
Compare
Add regression test for Mono crash when decoding custom attributes with generic enum arguments involving arrays (PR #123439). Uses GenericClassForEnum<int[]>.E to test the MONO_TYPE_GENERICINST handling in custom attribute decoding. Co-authored-by: Michal Strehovsky <MichalStrehovsky@users.noreply.github.com>
52f4a3f to
a33c5e6
Compare
|
@copilot update the description to reflect the current state of the pr |
Updated the PR description. The current PR now only contains the regression test (not the Mono runtime fix, which is in PR #123439). The test was also simplified to use |
Description
Adds regression test for custom attribute decoding with generic enum arguments involving arrays. This test validates the fix for a Mono runtime crash caused by incorrect MonoClass resolution for
MONO_TYPE_GENERICINSTwhen loading custom attribute values (see #123439).Note: This PR contains only the regression test. The actual Mono runtime fix is in PR #123439. This PR is meant to land after #123439 to provide test coverage.
Changes Made
Test Addition (
System.Reflection.Tests/CustomAttributeTests.cs):CustomAttributeCtor_WithGenericEnumArgument_DecodesCorrectlytest methodGenericClassForEnum<int[]>.Edecodes successfullyCustomAttributesandConstructorArgumentson decorated typesArgumentTypematches the expected generic enum typetypeof(GenericClassForEnum<int[]>.E)Valueequals the default enum value usingConvert.ToInt32()to properly compare the boxed enum value[ActiveIssue]attribute for a related bug in System.Reflection.Metadata/NativeAOT (CustomAttribute.DecodeValue doesn't handle generic type instantiations #123878)This test provides regression coverage for the issue, ensuring the crash doesn't reoccur after the fix in PR #123439 is merged.
Original prompt
This pull request was created from Copilot chat.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.